Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aditya/generator pcap #650

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Aditya/generator pcap #650

wants to merge 5 commits into from

Conversation

adigadi
Copy link
Contributor

@adigadi adigadi commented Sep 10, 2019

Added read/write to/from pcap files functionality

@gshimansky gshimansky changed the base branch from master to develop September 10, 2019 17:17
@gshimansky
Copy link
Contributor

gshimansky commented Sep 10, 2019

I see several problems with this approach. You're adding SetReceiverFile instead of SetReceiver and SetSenderFile instead of SetSender. This looks like this:

Original design:
 Generator           Test system
+----------+        +-----------+
| Receiver |<-------|           |
|          |        |           |
|  Sender  |------->|           |
+----------+        +-----------+

Design with file reader
    Generator              Test system
+----------------+        +-----------+
| Read from file |   ?<---|           |
|                |        |           |
|     Sender     |------->|           |
+----------------+        +-----------+

Design with file writer. BTW I don't think we need it in this form
    Generator              Test system
+----------------+        +-----------+
|    Receiver    |<-------|           |
|                |        |           |
| Write to file  |   ?--->|           |
+----------------+        +-----------+

You replaced Receive and Send in wrong places. What I meant by reading PCAP files is that generating function should read packets from PCAP instead of creating their contents randomly. Ideal place for PCAP file specification would be inside of JSON config file, e.g. instead of writing a generator config like this

{
    "ether": {
        "saddr": "00:25:96:FF:FE:12",
        "daddr": "00:FF:96:FF:FE:12",
        "ipv4": {
            "saddr": {
                "range": {
                    "min": "192.16.0.0",
                    "max": "192.16.0.255"
                }
            },
            "daddr": {
                "range": {
                    "min": "172.16.0.0",
                    "max": "172.16.0.10"
                }
            },
            "udp": {
                "sport": {
                    "range": {
                        "min": 1120,
                        "max": 1152
                    }
                },
                "dport": 1020,
                "raw": {
                    "data": "123456789012345678"
                }
            }
        }
    }
}

we could write something like

{
    "ether": {
        "pcap-file": "sample-network-sequence.pcap"
    }
}

In this case generating function would read packets from pcap file infinitely in a loop (no need for repeat count, packets number should be limited by packet rate and duration) and send them at the specified rate.

We could add file writer functionality. It would be useful for debugging requests like #645 where we need to answer a question, whether forwarded packets come reassembled or not. We need to not only receive packets, but also dump them into an output trace because tcpdump functionality is not available on DPDK ports. But writing packets into a file should be done only in addition to receiving them from the test application, not instead of it.

Several additional notes on your pull request:

  1. Please avoid whitespace changes. It is a good idea to run gofmt automatically on every file save which would always bring file to the coding convention that all Go code should be using. If you use VIM you can take a look at this link, for Emacs this link.
  2. There is no need to specify useReader and useWriter. These switches are redundant if you also specify file names. I think only file output switch should remain in this minPktLoss and perfTest applications while file reading directives should reside in JSON configuration for generator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants